Manual cherry-pick: [4.21] [Storage] refactor os matrix generator(#3883) by alexandermamaniy · Pull Request #4870 · RedHatQE/openshift-virtualization-tests

alexandermamaniy · 2026-05-14T22:46:52Z

Short description:

Backport of #3883 to cnv-4.21: refactor os matrix generator

More details:

move os matrix generators from global_config_*
files into a centralized location

What this PR does / why we need it:

This PR enables dynamic lookup via py_config["latest_instance_type_rhel_os_dict"], which is required to replace hardcoded RHEL10 preference and data source values in the rhel_vm_for_snapshot and rhel_vm_for_snapshot_with_content fixtures on s390x architecture.

This implementation is referenced in #4866, which requires architecture-aware RHEL preference support in snapshot fixtures when cherry-picked to cnv-4.21.

Which issue(s) this PR fixes:

Special notes for reviewer:

This backport centralizes OS matrix configuration to support architecture-specific preferences and data sources, preventing test failures on s390x where generic preferences enable incompatible firmware settings (SecureBoot/OVMF).

jira-ticket:

NONE

Signed-off-by: Alexander Mamani alexander.mamani@ibm.com

coderabbitai · 2026-05-14T22:46:58Z

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

🗂️ Base branches to auto review (4)

main
cnv-4.20
cnv-4.19
cnv-4.18

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 380aae93-269f-4383-beaa-4093df1c9d29

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

🔍 Trigger review

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

openshift-virtualization-qe-bot-5 · 2026-05-14T22:47:16Z

Report bugs in Issues

Welcome! 🎉

This pull request will be automatically processed with the following features:

🔄 Automatic Actions

Reviewer Assignment: Reviewers are automatically assigned based on the OWNERS file in the repository root
Size Labeling: PR size labels (XS, S, M, L, XL, XXL) are automatically applied based on changes
Issue Creation: A tracking issue is created for this PR and will be closed when the PR is merged or closed
Branch Labeling: Branch-specific labels are applied to track the target branch
Auto-verification: Auto-verified users have their PRs automatically marked as verified
Labels: Enabled categories: branch, can-be-merged, cherry-pick, has-conflicts, hold, needs-rebase, size, verified, wip

📋 Available Commands

PR Status Management

/wip - Mark PR as work in progress (adds WIP: prefix to title)
/wip cancel - Remove work in progress status
/hold - Block PR merging (approvers only)
/hold cancel - Unblock PR merging
/verified - Mark PR as verified
/verified cancel - Remove verification status
/reprocess - Trigger complete PR workflow reprocessing (useful if webhook failed or configuration changed)
/regenerate-welcome - Regenerate this welcome message

Review & Approval

/lgtm - Approve changes (looks good to me)
/approve - Approve PR (approvers only)
/assign-reviewers - Assign reviewers based on OWNERS file
/assign-reviewer @username - Assign specific reviewer
/check-can-merge - Check if PR meets merge requirements

Testing & Validation

/retest tox - Run Python test suite with tox
/retest build-container - Rebuild and test container image
/retest verify-bugs-are-open - verify-bugs-are-open
/retest all - Run all available tests

Container Operations

/build-and-push-container - Build and push container image (tagged with PR number)
- Supports additional build arguments: /build-and-push-container --build-arg KEY=value

Cherry-pick Operations

/cherry-pick <branch> - Schedule cherry-pick to target branch when PR is merged
- Multiple branches: /cherry-pick branch1 branch2 branch3

Label Management

/<label-name> - Add a label to the PR
/<label-name> cancel - Remove a label from the PR

✅ Merge Requirements

This PR will be automatically approved when the following conditions are met:

Approval: /approve from at least one approver
LGTM Count: Minimum 2 /lgtm from reviewers
Status Checks: All required status checks must pass
No Blockers: No wip, hold, has-conflicts labels and PR must be mergeable (no conflicts)
Verified: PR must be marked as verified

📊 Review Process

Approvers and Reviewers

Approvers:

dshchedr
myakove
rnetser
vsibirsk

Reviewers:

RoniKishner
dshchedr
geetikakay
rnetser
vsibirsk

Available Labels

hold
verified
wip
lgtm
approve

AI Features

Cherry-Pick Conflict Resolution: Enabled (claude/claude-opus-4-6[1m])

💡 Tips

WIP Status: Use /wip when your PR is not ready for review
Verification: The verified label is removed on new commits unless the push is detected as a clean rebase
Cherry-picking: Cherry-pick labels are processed when the PR is merged
Container Builds: Container images are automatically tagged with the PR number
Permission Levels: Some commands require approver permissions
Auto-verified Users: Certain users have automatic verification and merge privileges

For more information, please refer to the project documentation or contact the maintainers.

alexandermamaniy · 2026-06-22T13:35:33Z

/verified cancel

alexandermamaniy · 2026-06-22T13:36:48Z

/verified

openshift-virtualization-qe-bot-3 · 2026-06-22T13:37:22Z

@coderabbitai

Test execution plan request details

CRITICAL: You MUST post an inline review comment on the first changed line of the first file.
The inline comment should contain the full Test Execution Plan (smoke decision, gating decision, and specific affected tests).
Do NOT submit a blocking review event (REQUEST_CHANGES/APPROVE).
Post a single inline PR comment on Files Changed (non-blocking COMMENT flow).

As an expert software testing engineer, analyze all modified files in this PR and create a targeted test execution plan.
You will post an inline review comment with the test execution plan on the first changed file.
If you fail to run or post a comment, retry.

Analysis Requirements:

Examine code changes in each modified file
Identify affected code paths, functions, and classes
Analyze pytest-specific elements: fixtures (scope, dependencies), parametrization, markers, conftest changes
Trace test dependencies through imports, shared utilities, fixture inheritance, fixture teardown, and yield from cleanup in conftest
Detect new tests introduced in the PR
Utilities and libs impact (when utilities/ or libs/ changes):
You MUST use shell scripts (rg, git diff) to trace the full impact.
Follow these sub-steps in order:

6a. Identify modified symbols: For each changed file under utilities/ or libs/,
list every modified function or method.
Example: git diff HEAD~1 --unified=0 -- utilities/hco.py | grep '^[+-]def '

6b. Find direct callers: Search tests and conftest for each symbol from 6a.
Example: rg -l 'get_hco_version' tests/

6c. Trace fixture teardown and cleanup: Find fixtures that reach
the modified symbol through yield from or context-manager wrappers.
Example: rg -l 'yield from.*enable_common_boot|def.*enable_common_boot' tests/

6d. Trace same-file callers: In each changed file, find other functions
whose body calls a modified symbol (including code after yield
in @contextmanager helpers).
Example: rg 'get_hco_version|enable_common_boot' utilities/hco.py

6e. Expand transitively: If function A calls modified B, then
tests/fixtures that call A are affected — even when the test body
never imports B directly.

Do NOT limit impact to tests that import the modified symbol only.
Smoke test impact: Intersect the affected set from step 6 with smoke-marked tests.
Run: rg -l '@pytest.mark.smoke' tests/
VERIFY the above command returned actual file paths before concluding False.
Set True if either condition is met:
- a smoke-marked file appears in the affected set from 6b-6e, OR
- any conftest.py in the smoke test's parent-directory hierarchy (up to repo root)
  imports or calls a modified utilities/libs symbol — including autouse fixtures
  that depend on modified functions. ALL tests in that directory and below are affected.
  Example check: for each smoke_file, scan dirname(smoke_file)/conftest.py,
  dirname(dirname(smoke_file))/conftest.py, etc. for modified symbol imports
  and autouse fixtures that depend on modified symbols.
Gating test impact: Intersect the affected set from step 6 with gating-marked tests.
Run: rg -l '@pytest.mark.gating' tests/
Set True if a gating-marked file also appears in the affected set from 6b-6e.
Utilities/libs changes often affect gating tests without affecting smoke tests.
Do NOT stop analysis after concluding Run smoke tests: False.

Output rules:
Do NOT include analysis step numbers (1-8) in your visible output.

Your deliverable:
Your inline informational comment will be based on the following requirements:

Test Execution Plan

Run smoke tests: True / False — If True, state the dependency path (test → fixture → changed symbol). True ONLY with a verified path.
Run gating tests: True / False — If True, state the dependency path. True if any gating-marked test is in the affected set.
Affected tests to run (required when utilities/, libs/, or shared conftest changes — list concrete paths even when smoke is False)

Use these formats:

path/to/test_file.py - When the entire test file needs verification
path/to/test_file.py::TestClass::test_method - When specific test(s) needed
path/to/test_file.py::test_function - When specific test(s) needed
-m marker - When a marker covers multiple affected tests (e.g. -m gating only if ALL gating tests in scope need run)
Tag each listed test or group with its marker when not obvious, e.g. (gating) or (smoke)

Real test commands (MANDATORY when changes affect session/runtime code):

When the affected code runs at session/collection time (conftest fixtures, pytest plugins,
config hooks, session-scoped setup) or modifies runtime behavior that unit tests mock away,
you MUST include concrete pytest commands the PR author must run on a real cluster
to verify the change works end-to-end. Include:

A command for the error/fix path (the scenario the PR fixes)
A command for the happy path (regression: the normal case still works)
Use lightweight tests (e.g., --collect-only for startup failures,
a single small test for runtime behavior)
If the PR only changes test logic (not utilities/libs/conftest), the affected test
paths themselves serve as the real test commands — no separate section needed.

Example output for a session-startup fix:

**Real tests (cluster required)**
Error path (the fix):
`pytest tests/storage/.../test_foo.py --storage-class-matrix=nonexistent-sc --collect-only`
Expected: ValueError with clear message, not IndexError

Happy path (regression):
`pytest tests/storage/.../test_foo.py --storage-class-matrix=<valid-sc> -k test_bar`
Expected: session starts normally

Guidelines:

Include tests affected directly OR via fixture setup/teardown, yield from cleanup, or transitive utility call chains (caller calls modified helper)
Use a full file path only if ALL tests in that file require verification
Use file path + test name when only specific tests use an affected fixture or utility wrapper (preferred for partial file impact)
If a test marker can cover multiple files/tests, provide the marker
Balance coverage vs over-testing - Keep descriptions minimal
Example: if leaf helper foo() changes, include tests whose fixture teardown calls wrapper bar() where bar() calls foo(), even when the test body only imports an unrelated symbol from the same utilities module

Hardware-Related Checks (SR-IOV, GPU, DPDK):

When PR modifies fixtures for hardware-specific resources:

Collection Safety: Fixtures MUST have existence checks (return None when hardware unavailable)
Test Plan: MUST verify both WITH and WITHOUT hardware:
- Run affected tests on cluster WITH hardware
- Verify collection succeeds on cluster WITHOUT hardware

CRITICAL WORKFLOW COMPLETION RULES:

When responding to this test execution plan request, you MUST follow these rules EXACTLY:

YOUR ONLY DELIVERABLE: Post one non-blocking inline comment containing the test execution plan on the first changed line
THEN STOP IMMEDIATELY - Do NOT generate any additional response
FALLBACK ONLY: If inline comment API calls fail after retrying, post as a regular PR comment
SILENCE = SUCCESS: After successfully submitting the review, your task is complete. No confirmation needed.

ABSOLUTE PROHIBITIONS (violating these creates empty/meaningless reviews):

❌ Do NOT post acknowledgment messages like "Test execution plan posted", "Review posted successfully", "I've successfully posted"
❌ Do NOT mention review IDs, URLs, or confirmation of posting in the PR thread
❌ Do NOT add any follow-up comments after submitting the review
❌ Do NOT reply to confirm task completion
❌ Do NOT explain what you did - just do it and stop

Remember: The pull request review is visible to users. Additional comments are redundant noise.

CRITICAL — Verification rules for smoke and gating decisions:
- For each True decision, you must have traced a concrete path: test file → fixture → utility function → modified symbol.
- Do NOT assume impact based on scope, file proximity, or infrastructure-sounding names.
Only set True when you trace a concrete path through steps 6a-6e, then 7 or 8.
WRONG: "utilities/hco.py changed → run smoke" (no traced dependency path)
WRONG: "Run smoke tests: False" with no affected-test list when gating tests use the changed utility
RIGHT: "Run smoke tests: False, Run gating tests: True" plus tests/.../test_foo.py::TestBar::test_baz (gating)
RIGHT: "Smoke test X uses fixture Y → wrapper Z → changed function W in utilities/hco.py"

coderabbitai · 2026-06-22T13:38:14Z

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
gh pr view 4870 --repo RedHatQE/openshift-virtualization-tests --json headRefOid,files --jq '{sha: .headRefOid, files: [.files[].path]}'

Length of output: 550

🏁 Script executed:

#!/bin/bash
COMMIT_SHA="1401e3e1bf1b215a1a720f0f116ecf8b51c54c71"

BODY="**Test Execution Plan**

- **Run smoke tests: True**
  - \`tests/virt/cluster/common_templates/rhel/test_rhel_os_support.py\` (smoke) → \`py_config[\"rhel_os_matrix\"]\` → \`generate_os_matrix_dicts()\` in \`conftest.py::pytest_sessionstart\`
  - \`tests/infrastructure/instance_types/supported_os/test_rhel_os.py\` (smoke) → \`py_config[\"instance_type_rhel_os_matrix\"]\` → \`generate_os_matrix_dicts()\` in \`conftest.py::pytest_sessionstart\`
  - \`tests/storage/cdi_upload/test_upload_virtctl.py\` (smoke) → \`py_config[\"latest_rhel_os_dict\"]\` → \`update_latest_os_config()\` in \`conftest.py::pytest_sessionstart\`

- **Run gating tests: True**
  - \`tests/storage/cdi_upload/test_upload_virtctl.py\` (gating) → \`py_config[\"latest_rhel_os_dict\"]\` → \`update_latest_os_config()\` → \`generate_os_matrix_dicts()\` (refactored into \`utilities/pytest_utils.py\`)
  - \`tests/storage/cdi_import/test_import_http.py\` (gating) → \`py_config[\"latest_windows_os_dict\"]\` → \`update_latest_os_config()\` → \`generate_os_matrix_dicts()\`

**Affected tests to run:**

- \`tests/virt/cluster/common_templates/rhel/test_rhel_os_support.py\` (smoke)
- \`tests/infrastructure/instance_types/supported_os/test_rhel_os.py\` (smoke)
- \`tests/storage/cdi_upload/test_upload_virtctl.py\` (smoke + gating)
- \`tests/storage/cdi_import/test_import_http.py\` (gating)
- \`utilities/unittests/test_os_utils.py\`
- \`utilities/unittests/test_pytest_utils.py\`

**Real tests (cluster required)**

The refactored \`generate_os_matrix_dicts()\` and \`update_latest_os_config()\` run at session startup — verify end-to-end on a real cluster:

Error path (import chain after refactor — s390x arch-specific path):
\`\`\`
pytest tests/infrastructure/instance_types/supported_os/test_rhel_os.py --collect-only --tc-file=tests/global_config_s390x.py
\`\`\`
Expected: collection succeeds, \`py_config[\"instance_type_rhel_os_matrix\"]\` populated with s390x-specific preferences (no \`KeyError\` / \`ImportError\`)

Happy path (regression — x86_64, OS matrix values correct):
\`\`\`
pytest tests/virt/cluster/common_templates/rhel/test_rhel_os_support.py --collect-only --tc-file=tests/global_config_x86_64.py
\`\`\`
Expected: parametrized test IDs match expected RHEL OS versions for x86_64"

gh api \
  --method POST \
  -H "Accept: application/vnd.github+json" \
  /repos/RedHatQE/openshift-virtualization-tests/pulls/4870/reviews \
  -f commit_id="$COMMIT_SHA" \
  -f event="COMMENT" \
  -f "comments[][path]=conftest.py" \
  -F "comments[][line]=30" \
  -f "comments[][side]=RIGHT" \
  -f "comments[][body]=$BODY"

Length of output: 4459

move os matrix generators from global_config_* files into a centralized location Original PR: RedHatQE#3883 Signed-off-by: Alexander Mamani <alexander.mamani@ibm.com>

- utilities/pytest_utils.py: Remove unused mark_nmstate_dependent_tests function Signed-off-by: Alexander Mamani <alexander.mamani@ibm.com>

Signed-off-by: Alexander Mamani <alexander.mamani@ibm.com>

Since in OCP 4.21 is still using X86_64 as suffix for preference and unittest are using AMD_64 suffix. Make cpu_arch flexible for both Signed-off-by: Alexander Mamani <alexander.mamani@ibm.com>

openshift-virtualization-qe-bot · 2026-06-22T21:24:02Z

Clean rebase detected — no code changes compared to previous head (1401e3e).
The following labels were preserved: verified, lgtm-vsibirsk, lgtm-rnetser, commented-coderabbitai[bot], approved-rnetser, lgtm-coderabbitai[bot], commented-openshift-virtualization-qe-bot-3, commented-alexandermamaniy.

alexandermamaniy · 2026-06-22T21:30:18Z

/verified cancel

alexandermamaniy · 2026-06-22T21:31:09Z

/verified

openshift-virtualization-qe-bot-3 · 2026-06-22T21:31:40Z

@coderabbitai

Test execution plan request details

CRITICAL: You MUST post an inline review comment on the first changed line of the first file.
The inline comment should contain the full Test Execution Plan (smoke decision, gating decision, and specific affected tests).
Do NOT submit a blocking review event (REQUEST_CHANGES/APPROVE).
Post a single inline PR comment on Files Changed (non-blocking COMMENT flow).

As an expert software testing engineer, analyze all modified files in this PR and create a targeted test execution plan.
You will post an inline review comment with the test execution plan on the first changed file.
If you fail to run or post a comment, retry.

Analysis Requirements:

Examine code changes in each modified file
Identify affected code paths, functions, and classes
Analyze pytest-specific elements: fixtures (scope, dependencies), parametrization, markers, conftest changes
Trace test dependencies through imports, shared utilities, fixture inheritance, fixture teardown, and yield from cleanup in conftest
Detect new tests introduced in the PR
Utilities and libs impact (when utilities/ or libs/ changes):
You MUST use shell scripts (rg, git diff) to trace the full impact.
Follow these sub-steps in order:

6a. Identify modified symbols: For each changed file under utilities/ or libs/,
list every modified function or method.
Example: git diff HEAD~1 --unified=0 -- utilities/hco.py | grep '^[+-]def '

6b. Find direct callers: Search tests and conftest for each symbol from 6a.
Example: rg -l 'get_hco_version' tests/

6c. Trace fixture teardown and cleanup: Find fixtures that reach
the modified symbol through yield from or context-manager wrappers.
Example: rg -l 'yield from.*enable_common_boot|def.*enable_common_boot' tests/

6d. Trace same-file callers: In each changed file, find other functions
whose body calls a modified symbol (including code after yield
in @contextmanager helpers).
Example: rg 'get_hco_version|enable_common_boot' utilities/hco.py

6e. Expand transitively: If function A calls modified B, then
tests/fixtures that call A are affected — even when the test body
never imports B directly.

Do NOT limit impact to tests that import the modified symbol only.
Smoke test impact: Intersect the affected set from step 6 with smoke-marked tests.
Run: rg -l '@pytest.mark.smoke' tests/
VERIFY the above command returned actual file paths before concluding False.
Set True if either condition is met:
- a smoke-marked file appears in the affected set from 6b-6e, OR
- any conftest.py in the smoke test's parent-directory hierarchy (up to repo root)
  imports or calls a modified utilities/libs symbol — including autouse fixtures
  that depend on modified functions. ALL tests in that directory and below are affected.
  Example check: for each smoke_file, scan dirname(smoke_file)/conftest.py,
  dirname(dirname(smoke_file))/conftest.py, etc. for modified symbol imports
  and autouse fixtures that depend on modified symbols.
Gating test impact: Intersect the affected set from step 6 with gating-marked tests.
Run: rg -l '@pytest.mark.gating' tests/
Set True if a gating-marked file also appears in the affected set from 6b-6e.
Utilities/libs changes often affect gating tests without affecting smoke tests.
Do NOT stop analysis after concluding Run smoke tests: False.

Output rules:
Do NOT include analysis step numbers (1-8) in your visible output.

Your deliverable:
Your inline informational comment will be based on the following requirements:

Test Execution Plan

Run smoke tests: True / False — If True, state the dependency path (test → fixture → changed symbol). True ONLY with a verified path.
Run gating tests: True / False — If True, state the dependency path. True if any gating-marked test is in the affected set.
Affected tests to run (required when utilities/, libs/, or shared conftest changes — list concrete paths even when smoke is False)

Use these formats:

path/to/test_file.py - When the entire test file needs verification
path/to/test_file.py::TestClass::test_method - When specific test(s) needed
path/to/test_file.py::test_function - When specific test(s) needed
-m marker - When a marker covers multiple affected tests (e.g. -m gating only if ALL gating tests in scope need run)
Tag each listed test or group with its marker when not obvious, e.g. (gating) or (smoke)

Real test commands (MANDATORY when changes affect session/runtime code):

When the affected code runs at session/collection time (conftest fixtures, pytest plugins,
config hooks, session-scoped setup) or modifies runtime behavior that unit tests mock away,
you MUST include concrete pytest commands the PR author must run on a real cluster
to verify the change works end-to-end. Include:

A command for the error/fix path (the scenario the PR fixes)
A command for the happy path (regression: the normal case still works)
Use lightweight tests (e.g., --collect-only for startup failures,
a single small test for runtime behavior)
If the PR only changes test logic (not utilities/libs/conftest), the affected test
paths themselves serve as the real test commands — no separate section needed.

Example output for a session-startup fix:

**Real tests (cluster required)**
Error path (the fix):
`pytest tests/storage/.../test_foo.py --storage-class-matrix=nonexistent-sc --collect-only`
Expected: ValueError with clear message, not IndexError

Happy path (regression):
`pytest tests/storage/.../test_foo.py --storage-class-matrix=<valid-sc> -k test_bar`
Expected: session starts normally

Guidelines:

Include tests affected directly OR via fixture setup/teardown, yield from cleanup, or transitive utility call chains (caller calls modified helper)
Use a full file path only if ALL tests in that file require verification
Use file path + test name when only specific tests use an affected fixture or utility wrapper (preferred for partial file impact)
If a test marker can cover multiple files/tests, provide the marker
Balance coverage vs over-testing - Keep descriptions minimal
Example: if leaf helper foo() changes, include tests whose fixture teardown calls wrapper bar() where bar() calls foo(), even when the test body only imports an unrelated symbol from the same utilities module

Hardware-Related Checks (SR-IOV, GPU, DPDK):

When PR modifies fixtures for hardware-specific resources:

Collection Safety: Fixtures MUST have existence checks (return None when hardware unavailable)
Test Plan: MUST verify both WITH and WITHOUT hardware:
- Run affected tests on cluster WITH hardware
- Verify collection succeeds on cluster WITHOUT hardware

CRITICAL WORKFLOW COMPLETION RULES:

When responding to this test execution plan request, you MUST follow these rules EXACTLY:

YOUR ONLY DELIVERABLE: Post one non-blocking inline comment containing the test execution plan on the first changed line
THEN STOP IMMEDIATELY - Do NOT generate any additional response
FALLBACK ONLY: If inline comment API calls fail after retrying, post as a regular PR comment
SILENCE = SUCCESS: After successfully submitting the review, your task is complete. No confirmation needed.

ABSOLUTE PROHIBITIONS (violating these creates empty/meaningless reviews):

❌ Do NOT post acknowledgment messages like "Test execution plan posted", "Review posted successfully", "I've successfully posted"
❌ Do NOT mention review IDs, URLs, or confirmation of posting in the PR thread
❌ Do NOT add any follow-up comments after submitting the review
❌ Do NOT reply to confirm task completion
❌ Do NOT explain what you did - just do it and stop

Remember: The pull request review is visible to users. Additional comments are redundant noise.

CRITICAL — Verification rules for smoke and gating decisions:
- For each True decision, you must have traced a concrete path: test file → fixture → utility function → modified symbol.
- Do NOT assume impact based on scope, file proximity, or infrastructure-sounding names.
Only set True when you trace a concrete path through steps 6a-6e, then 7 or 8.
WRONG: "utilities/hco.py changed → run smoke" (no traced dependency path)
WRONG: "Run smoke tests: False" with no affected-test list when gating tests use the changed utility
RIGHT: "Run smoke tests: False, Run gating tests: True" plus tests/.../test_foo.py::TestBar::test_baz (gating)
RIGHT: "Smoke test X uses fixture Y → wrapper Z → changed function W in utilities/hco.py"

coderabbitai · 2026-06-22T21:32:34Z

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
gh pr view 4870 --repo RedHatQE/openshift-virtualization-tests --json headRefOid,files --jq '{sha: .headRefOid, files: [.files[].path]}'

Length of output: 550

🏁 Script executed:

#!/bin/bash
COMMIT_SHA="a27819d3396a7ec947283d9a3a734fa2afa137f0"

BODY="**Test Execution Plan**

- **Run smoke tests: True**
  - \`tests/virt/cluster/common_templates/rhel/test_rhel_os_support.py\` (smoke) → \`py_config[\"rhel_os_matrix\"]\` → \`generate_os_matrix_dicts()\` in \`conftest.py::pytest_sessionstart\`
  - \`tests/infrastructure/instance_types/supported_os/test_rhel_os.py\` (smoke) → \`py_config[\"instance_type_rhel_os_matrix\"]\` → \`generate_os_matrix_dicts()\` in \`conftest.py::pytest_sessionstart\`
  - \`tests/storage/cdi_upload/test_upload_virtctl.py\` (smoke) → \`py_config[\"latest_rhel_os_dict\"]\` → \`update_latest_os_config()\` in \`conftest.py::pytest_sessionstart\`

- **Run gating tests: True**
  - \`tests/storage/cdi_upload/test_upload_virtctl.py\` (gating) → \`py_config[\"latest_rhel_os_dict\"]\` → \`update_latest_os_config()\` → \`generate_os_matrix_dicts()\` (refactored into \`utilities/pytest_utils.py\`)
  - \`tests/storage/cdi_import/test_import_http.py\` (gating) → \`py_config[\"latest_windows_os_dict\"]\` → \`update_latest_os_config()\` → \`generate_os_matrix_dicts()\`

**Affected tests to run:**

- \`tests/virt/cluster/common_templates/rhel/test_rhel_os_support.py\` (smoke)
- \`tests/infrastructure/instance_types/supported_os/test_rhel_os.py\` (smoke)
- \`tests/storage/cdi_upload/test_upload_virtctl.py\` (smoke + gating)
- \`tests/storage/cdi_import/test_import_http.py\` (gating)
- \`utilities/unittests/test_os_utils.py\`
- \`utilities/unittests/test_pytest_utils.py\`

**Real tests (cluster required)**

The refactored \`generate_os_matrix_dicts()\` and \`update_latest_os_config()\` run at session startup via \`conftest.py::pytest_sessionstart\` — verify end-to-end on a real cluster:

Error path (import chain after refactor — s390x arch-specific path):
\`\`\`
pytest tests/infrastructure/instance_types/supported_os/test_rhel_os.py --collect-only --tc-file=tests/global_config_s390x.py
\`\`\`
Expected: collection succeeds, \`py_config[\"instance_type_rhel_os_matrix\"]\` populated with s390x-specific preferences (no \`KeyError\` / \`ImportError\`)

Happy path (regression — x86_64, OS matrix values correct):
\`\`\`
pytest tests/virt/cluster/common_templates/rhel/test_rhel_os_support.py --collect-only --tc-file=tests/global_config_x86_64.py
\`\`\`
Expected: parametrized test IDs match expected RHEL OS versions for x86_64"

gh api \
  --method POST \
  -H "Accept: application/vnd.github+json" \
  /repos/RedHatQE/openshift-virtualization-tests/pulls/4870/reviews \
  -f commit_id="$COMMIT_SHA" \
  -f event="COMMENT" \
  -f "comments[][path]=conftest.py" \
  -F "comments[][line]=30" \
  -f "comments[][side]=RIGHT" \
  -f "comments[][body]=$BODY"

Length of output: 4500

alexandermamaniy

/verified

alexandermamaniy · 2026-06-23T07:47:13Z

/reprocess

alexandermamaniy · 2026-06-23T08:04:42Z

/verified

openshift-virtualization-qe-bot-3 · 2026-06-23T08:05:17Z

@coderabbitai

Test execution plan request details

CRITICAL: You MUST post an inline review comment on the first changed line of the first file.
The inline comment should contain the full Test Execution Plan (smoke decision, gating decision, and specific affected tests).
Do NOT submit a blocking review event (REQUEST_CHANGES/APPROVE).
Post a single inline PR comment on Files Changed (non-blocking COMMENT flow).

As an expert software testing engineer, analyze all modified files in this PR and create a targeted test execution plan.
You will post an inline review comment with the test execution plan on the first changed file.
If you fail to run or post a comment, retry.

Analysis Requirements:

Examine code changes in each modified file
Identify affected code paths, functions, and classes
Analyze pytest-specific elements: fixtures (scope, dependencies), parametrization, markers, conftest changes
Trace test dependencies through imports, shared utilities, fixture inheritance, fixture teardown, and yield from cleanup in conftest
Detect new tests introduced in the PR
Utilities and libs impact (when utilities/ or libs/ changes):
You MUST use shell scripts (rg, git diff) to trace the full impact.
Follow these sub-steps in order:

6a. Identify modified symbols: For each changed file under utilities/ or libs/,
list every modified function or method.
Example: git diff HEAD~1 --unified=0 -- utilities/hco.py | grep '^[+-]def '

6b. Find direct callers: Search tests and conftest for each symbol from 6a.
Example: rg -l 'get_hco_version' tests/

6c. Trace fixture teardown and cleanup: Find fixtures that reach
the modified symbol through yield from or context-manager wrappers.
Example: rg -l 'yield from.*enable_common_boot|def.*enable_common_boot' tests/

6d. Trace same-file callers: In each changed file, find other functions
whose body calls a modified symbol (including code after yield
in @contextmanager helpers).
Example: rg 'get_hco_version|enable_common_boot' utilities/hco.py

6e. Expand transitively: If function A calls modified B, then
tests/fixtures that call A are affected — even when the test body
never imports B directly.

Do NOT limit impact to tests that import the modified symbol only.
Smoke test impact: Intersect the affected set from step 6 with smoke-marked tests.
Run: rg -l '@pytest.mark.smoke' tests/
VERIFY the above command returned actual file paths before concluding False.
Set True if either condition is met:
- a smoke-marked file appears in the affected set from 6b-6e, OR
- any conftest.py in the smoke test's parent-directory hierarchy (up to repo root)
  imports or calls a modified utilities/libs symbol — including autouse fixtures
  that depend on modified functions. ALL tests in that directory and below are affected.
  Example check: for each smoke_file, scan dirname(smoke_file)/conftest.py,
  dirname(dirname(smoke_file))/conftest.py, etc. for modified symbol imports
  and autouse fixtures that depend on modified symbols.
Gating test impact: Intersect the affected set from step 6 with gating-marked tests.
Run: rg -l '@pytest.mark.gating' tests/
Set True if a gating-marked file also appears in the affected set from 6b-6e.
Utilities/libs changes often affect gating tests without affecting smoke tests.
Do NOT stop analysis after concluding Run smoke tests: False.

Output rules:
Do NOT include analysis step numbers (1-8) in your visible output.

Your deliverable:
Your inline informational comment will be based on the following requirements:

Test Execution Plan

Run smoke tests: True / False — If True, state the dependency path (test → fixture → changed symbol). True ONLY with a verified path.
Run gating tests: True / False — If True, state the dependency path. True if any gating-marked test is in the affected set.
Affected tests to run (required when utilities/, libs/, or shared conftest changes — list concrete paths even when smoke is False)

Use these formats:

path/to/test_file.py - When the entire test file needs verification
path/to/test_file.py::TestClass::test_method - When specific test(s) needed
path/to/test_file.py::test_function - When specific test(s) needed
-m marker - When a marker covers multiple affected tests (e.g. -m gating only if ALL gating tests in scope need run)
Tag each listed test or group with its marker when not obvious, e.g. (gating) or (smoke)

Real test commands (MANDATORY when changes affect session/runtime code):

When the affected code runs at session/collection time (conftest fixtures, pytest plugins,
config hooks, session-scoped setup) or modifies runtime behavior that unit tests mock away,
you MUST include concrete pytest commands the PR author must run on a real cluster
to verify the change works end-to-end. Include:

A command for the error/fix path (the scenario the PR fixes)
A command for the happy path (regression: the normal case still works)
Use lightweight tests (e.g., --collect-only for startup failures,
a single small test for runtime behavior)
If the PR only changes test logic (not utilities/libs/conftest), the affected test
paths themselves serve as the real test commands — no separate section needed.

Example output for a session-startup fix:

**Real tests (cluster required)**
Error path (the fix):
`pytest tests/storage/.../test_foo.py --storage-class-matrix=nonexistent-sc --collect-only`
Expected: ValueError with clear message, not IndexError

Happy path (regression):
`pytest tests/storage/.../test_foo.py --storage-class-matrix=<valid-sc> -k test_bar`
Expected: session starts normally

Guidelines:

Include tests affected directly OR via fixture setup/teardown, yield from cleanup, or transitive utility call chains (caller calls modified helper)
Use a full file path only if ALL tests in that file require verification
Use file path + test name when only specific tests use an affected fixture or utility wrapper (preferred for partial file impact)
If a test marker can cover multiple files/tests, provide the marker
Balance coverage vs over-testing - Keep descriptions minimal
Example: if leaf helper foo() changes, include tests whose fixture teardown calls wrapper bar() where bar() calls foo(), even when the test body only imports an unrelated symbol from the same utilities module

Hardware-Related Checks (SR-IOV, GPU, DPDK):

When PR modifies fixtures for hardware-specific resources:

Collection Safety: Fixtures MUST have existence checks (return None when hardware unavailable)
Test Plan: MUST verify both WITH and WITHOUT hardware:
- Run affected tests on cluster WITH hardware
- Verify collection succeeds on cluster WITHOUT hardware

CRITICAL WORKFLOW COMPLETION RULES:

When responding to this test execution plan request, you MUST follow these rules EXACTLY:

YOUR ONLY DELIVERABLE: Post one non-blocking inline comment containing the test execution plan on the first changed line
THEN STOP IMMEDIATELY - Do NOT generate any additional response
FALLBACK ONLY: If inline comment API calls fail after retrying, post as a regular PR comment
SILENCE = SUCCESS: After successfully submitting the review, your task is complete. No confirmation needed.

ABSOLUTE PROHIBITIONS (violating these creates empty/meaningless reviews):

❌ Do NOT post acknowledgment messages like "Test execution plan posted", "Review posted successfully", "I've successfully posted"
❌ Do NOT mention review IDs, URLs, or confirmation of posting in the PR thread
❌ Do NOT add any follow-up comments after submitting the review
❌ Do NOT reply to confirm task completion
❌ Do NOT explain what you did - just do it and stop

Remember: The pull request review is visible to users. Additional comments are redundant noise.

CRITICAL — Verification rules for smoke and gating decisions:
- For each True decision, you must have traced a concrete path: test file → fixture → utility function → modified symbol.
- Do NOT assume impact based on scope, file proximity, or infrastructure-sounding names.
Only set True when you trace a concrete path through steps 6a-6e, then 7 or 8.
WRONG: "utilities/hco.py changed → run smoke" (no traced dependency path)
WRONG: "Run smoke tests: False" with no affected-test list when gating tests use the changed utility
RIGHT: "Run smoke tests: False, Run gating tests: True" plus tests/.../test_foo.py::TestBar::test_baz (gating)
RIGHT: "Smoke test X uses fixture Y → wrapper Z → changed function W in utilities/hco.py"

coderabbitai · 2026-06-23T08:06:18Z

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
gh pr view 4870 --repo RedHatQE/openshift-virtualization-tests --json headRefOid,files --jq '{sha: .headRefOid, files: [.files[].path]}'

Length of output: 644

🏁 Script executed:

#!/bin/bash
# Get first changed line in conftest.py
gh pr diff 4870 --repo RedHatQE/openshift-virtualization-tests -- conftest.py 2>/dev/null | head -60

COMMIT_SHA="a27819d3396a7ec947283d9a3a734fa2afa137f0"

BODY="**Test Execution Plan**

- **Run smoke tests: True**
  - \`tests/virt/cluster/common_templates/rhel/test_rhel_os_support.py\` (smoke) → \`py_config[\"rhel_os_matrix\"]\` → \`generate_os_matrix_dicts()\` in \`conftest.py::pytest_sessionstart\`
  - \`tests/infrastructure/instance_types/supported_os/test_rhel_os.py\` (smoke) → \`py_config[\"instance_type_rhel_os_matrix\"]\` → \`generate_os_matrix_dicts()\` in \`conftest.py::pytest_sessionstart\`
  - \`tests/storage/cdi_upload/test_upload_virtctl.py\` (smoke) → \`py_config[\"latest_rhel_os_dict\"]\` → \`update_latest_os_config()\` in \`conftest.py::pytest_sessionstart\`

- **Run gating tests: True**
  - \`tests/storage/cdi_upload/test_upload_virtctl.py\` (gating) → \`py_config[\"latest_rhel_os_dict\"]\` → \`update_latest_os_config()\` → \`generate_os_matrix_dicts()\` (refactored into \`utilities/pytest_utils.py\`)
  - \`tests/storage/cdi_import/test_import_http.py\` (gating) → \`py_config[\"latest_windows_os_dict\"]\` → \`update_latest_os_config()\` → \`generate_os_matrix_dicts()\`

**Affected tests to run:**

- \`tests/virt/cluster/common_templates/rhel/test_rhel_os_support.py\` (smoke)
- \`tests/infrastructure/instance_types/supported_os/test_rhel_os.py\` (smoke)
- \`tests/storage/cdi_upload/test_upload_virtctl.py\` (smoke + gating)
- \`tests/storage/cdi_import/test_import_http.py\` (gating)
- \`utilities/unittests/test_os_utils.py\`
- \`utilities/unittests/test_pytest_utils.py\`

**Real tests (cluster required)**

The refactored \`generate_os_matrix_dicts()\` and \`update_latest_os_config()\` run at session startup via \`conftest.py::pytest_sessionstart\` — verify end-to-end on a real cluster:

Error path (import chain after refactor — s390x arch-specific path):
\`\`\`
pytest tests/infrastructure/instance_types/supported_os/test_rhel_os.py --collect-only --tc-file=tests/global_config_s390x.py
\`\`\`
Expected: collection succeeds, \`py_config[\"instance_type_rhel_os_matrix\"]\` populated with s390x-specific preferences (no \`KeyError\` / \`ImportError\`)

Happy path (regression — x86_64, OS matrix values correct):
\`\`\`
pytest tests/virt/cluster/common_templates/rhel/test_rhel_os_support.py --collect-only --tc-file=tests/global_config_x86_64.py
\`\`\`
Expected: parametrized test IDs match expected RHEL OS versions for x86_64"

# Get the diff to find the exact first changed line
DIFF_OUTPUT=$(gh pr diff 4870 --repo RedHatQE/openshift-virtualization-tests -- conftest.py 2>/dev/null)
echo "$DIFF_OUTPUT" | grep -n "^@@" | head -5
# Extract the first new line number from the diff header
FIRST_LINE=$(echo "$DIFF_OUTPUT" | grep "^@@" | head -1 | grep -oP '\+\K[0-9]+')
echo "First changed line: $FIRST_LINE"

gh api \
  --method POST \
  -H "Accept: application/vnd.github+json" \
  /repos/RedHatQE/openshift-virtualization-tests/pulls/4870/reviews \
  -f commit_id="$COMMIT_SHA" \
  -f event="COMMENT" \
  -f "comments[][path]=conftest.py" \
  -F "comments[][line]=${FIRST_LINE:-30}" \
  -f "comments[][side]=RIGHT" \
  -f "comments[][body]=$BODY"

Length of output: 5153

openshift-virtualization-qe-bot-3 · 2026-06-24T08:27:01Z

/retest all

Auto-triggered: Files in this PR were modified by merged PR #5332.

Overlapping files

conftest.py

rnetser · 2026-06-24T18:33:05Z

@alexandermamaniy please add how was the PR verified

alexandermamaniy · 2026-06-25T08:31:32Z

/verified

Collection of tests on s390x
uv run pytest --verbose -rs -s -o log_cli=true --leftovers-collector --tb=native --skip-deprecated-api-test tests/infrastructure/instance_types/supported_os/test_rhel_os.py --collect-only --tc-file=tests/global_config_s390x.py
48/49 tests collected (1 deselected) in 0.08s

rnetser · 2026-06-25T14:34:11Z

/verified

Collection of tests on s390x uv run pytest --verbose -rs -s -o log_cli=true --leftovers-collector --tb=native --skip-deprecated-api-test tests/infrastructure/instance_types/supported_os/test_rhel_os.py --collect-only --tc-file=tests/global_config_s390x.py 48/49 tests collected (1 deselected) in 0.08s

and also https://github.com/RedHatQE/openshift-virtualization-tests/pull/4870/files#r3457848925?
collection only is not enough

alexandermamaniy · 2026-06-25T22:50:08Z

/verified

Test verification on s390x

uv run pytest --verbose --log-cli-level=true -m 'tier2 and storage and gating' -svv --latest-rhel --storage-class-matrix=ocs-storagecluster-ceph-rbd-virtualization tests/storage --leftovers-collector --tb=native --tc-file=tests/global_config_s390x.py

_ 22 of 22 completed, 22 Pass, 0 Fail, 0 Skip, 0 XPass, 0 XFail, 0 Error, 0 ReRun _
======== 22 passed, 222 deselected, 249 warnings in 2470.15s (0:41:10) =========

rnetser · 2026-06-28T11:55:01Z

/verified

Test verification on s390x

uv run pytest --verbose --log-cli-level=true -m 'tier2 and storage and gating' -svv --latest-rhel --storage-class-matrix=ocs-storagecluster-ceph-rbd-virtualization tests/storage --leftovers-collector --tb=native --tc-file=tests/global_config_s390x.py

_ 22 of 22 completed, 22 Pass, 0 Fail, 0 Skip, 0 XPass, 0 XFail, 0 Error, 0 ReRun _ ======== 22 passed, 222 deselected, 249 warnings in 2470.15s (0:41:10) =========

Thank you, please note that as this change also affects other teams, please also run -m gating tests/virt

rnetser · 2026-06-29T09:00:45Z

/approve cancel

removing approval until #4870 (comment) is addressed to avoid merge by mistake

openshift-virtualization-qe-bot-3 · 2026-06-30T02:03:15Z

/retest all

Auto-triggered: Files in this PR were modified by merged PR #5169.

Overlapping files

conftest.py

openshift-virtualization-qe-bot-5 requested a review from vsibirsk May 14, 2026 22:47

openshift-virtualization-qe-bot-5 assigned alexandermamaniy May 14, 2026

openshift-virtualization-qe-bot-5 requested a review from RoniKishner May 14, 2026 22:47

openshift-virtualization-qe-bot-5 added the branch-cnv-4.21 label May 14, 2026

openshift-virtualization-qe-bot-5 requested review from dshchedr and rnetser May 14, 2026 22:47

openshift-virtualization-qe-bot-5 added the size/XXL label May 14, 2026

openshift-virtualization-qe-bot-5 requested a review from geetikakay May 14, 2026 22:47

openshift-virtualization-qe-bot added tox:verify-tc-requirement-polarion:running D/S Testing tox:verify-tc-requirement-polarion:passed D/S Testing and removed tox:verify-tc-requirement-polarion:running D/S Testing tox:verify-tc-requirement-polarion:passed D/S Testing labels May 14, 2026

alexandermamaniy marked this pull request as draft May 15, 2026 07:24

alexandermamaniy force-pushed the backport/4.21/vk-refactor-matrix-generator branch from 4e7601a to 98b26de Compare May 18, 2026 00:40

openshift-virtualization-qe-bot added tox:verify-tc-requirement-polarion:running D/S Testing tox:verify-tc-requirement-polarion:passed D/S Testing and removed tox:verify-tc-requirement-polarion:passed D/S Testing tox:verify-tc-requirement-polarion:running D/S Testing labels May 18, 2026

alexandermamaniy commented Jun 22, 2026

View reviewed changes

Comment thread conftest.py

Comment thread tests/global_config_x86_64.py

Comment thread conftest.py

coderabbitai Bot reviewed Jun 22, 2026

View reviewed changes

Comment thread conftest.py

vsibirsk and others added 4 commits June 22, 2026 22:23

refactor os matrix generator

d067631

move os matrix generators from global_config_* files into a centralized location Original PR: RedHatQE#3883 Signed-off-by: Alexander Mamani <alexander.mamani@ibm.com>

refactor: remove mark_nmstate_dependent_tests function

ae9dc82

- utilities/pytest_utils.py: Remove unused mark_nmstate_dependent_tests function Signed-off-by: Alexander Mamani <alexander.mamani@ibm.com>

fix: Add missed win-2016 os to windows_os_list

f139983

Signed-off-by: Alexander Mamani <alexander.mamani@ibm.com>

fix: set right cpu_arch for unittest and test

a27819d

Since in OCP 4.21 is still using X86_64 as suffix for preference and unittest are using AMD_64 suffix. Make cpu_arch flexible for both Signed-off-by: Alexander Mamani <alexander.mamani@ibm.com>

Uh oh!

Conversation

alexandermamaniy commented May 14, 2026

Short description:

More details:

What this PR does / why we need it:

Which issue(s) this PR fixes:

Special notes for reviewer:

jira-ticket:

Uh oh!

coderabbitai Bot commented May 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review skipped

Uh oh!

openshift-virtualization-qe-bot-5 commented May 14, 2026

Welcome! 🎉

🔄 Automatic Actions

📋 Available Commands

PR Status Management

Review & Approval

Testing & Validation

Container Operations

Cherry-pick Operations

Label Management

✅ Merge Requirements

📊 Review Process

💡 Tips

Uh oh!

Uh oh!

Uh oh!

Uh oh!

alexandermamaniy commented Jun 22, 2026

Uh oh!

alexandermamaniy commented Jun 22, 2026

Uh oh!

openshift-virtualization-qe-bot-3 commented Jun 22, 2026

Uh oh!

Uh oh!

coderabbitai Bot commented Jun 22, 2026

Uh oh!

openshift-virtualization-qe-bot commented Jun 22, 2026

Uh oh!

alexandermamaniy commented Jun 22, 2026

Uh oh!

alexandermamaniy commented Jun 22, 2026

Uh oh!

openshift-virtualization-qe-bot-3 commented Jun 22, 2026

Uh oh!

Uh oh!

coderabbitai Bot commented Jun 22, 2026

Uh oh!

alexandermamaniy left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

alexandermamaniy commented Jun 23, 2026

Uh oh!

alexandermamaniy commented Jun 23, 2026

Uh oh!

openshift-virtualization-qe-bot-3 commented Jun 23, 2026

Uh oh!

Uh oh!

coderabbitai Bot commented Jun 23, 2026

Uh oh!

openshift-virtualization-qe-bot-3 commented Jun 24, 2026

Uh oh!

rnetser commented Jun 24, 2026

Uh oh!

alexandermamaniy commented Jun 25, 2026

Uh oh!

rnetser commented Jun 25, 2026

Uh oh!

alexandermamaniy commented Jun 25, 2026

Uh oh!

rnetser commented Jun 28, 2026

Uh oh!

rnetser commented Jun 29, 2026

Uh oh!

openshift-virtualization-qe-bot-3 commented Jun 30, 2026

Uh oh!

coderabbitai Bot commented May 14, 2026 •

edited

Loading

alexandermamaniy left a comment •

edited

Loading